Go: Fix multiple bugs in rewrite-go module#7212
Merged
jkschneider merged 1 commit intomainfrom Mar 31, 2026
Merged
Conversation
Motivation: Code review of the rewrite-go module identified several bugs and missing visitor coverage that would impact correctness and robustness. Summary: - Fix panic recovery in safeHandleRequest to return a proper JSON-RPC error response instead of nil when a panic is caught - Fix parser to handle multiple import blocks (e.g., `import "fmt"` followed by `import "os"`), using a new ImportBlock marker to track block boundaries - Fix visitAndCast and visitExpression to handle nil returns from visitors instead of panicking on the type assertion - Fix VisitCompilationUnit to visit PackageDecl and Imports, which were previously skipped entirely (preventing recipe visitors from transforming package names or imports) - Remove dead code in mapArrayType (unreachable closePrefix computation)
knutwannheden
changed the title
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Motivation
Code review of the rewrite-go module identified several bugs and missing visitor coverage that would impact correctness and robustness of the Go language support.
Summary
safeHandleRequestnow returns a proper JSON-RPC error response instead ofnilwhen a panic is caught, preventing silent session deathimport "fmt"followed byimport "os"(common in generated code). The parser previously only processed the first import block, corrupting cursor position tracking for the rest of the file. A newImportBlockmarker tracks block boundaries, with corresponding printer, RPC serialization, and Java marker support.visitAndCast/visitExpressionnil panics — These helpers now handlenilreturns from visitors instead of panicking on the type assertionVisitCompilationUnitto visit PackageDecl and Imports — These were previously skipped entirely, meaning no recipe visitor could transform package names or importsmapArrayType— UnreachableclosePrefixcomputation that was immediately overwrittenTest plan
TestParseMultipleImportBlocks— parse-print round-trip forimport "fmt"/import "os"TestParseMultipleGroupedImportBlocks— parse-print round-trip for multipleimport (...)blocksTestVisitorReturningNilDoesNotPanic— visitor returning nil for a Return node doesn't crashTestVisitorVisitsImports— confirms import nodes are visited (was 0, now 2)TestVisitorVisitsPackageDecl— confirms package decl identifier is visited